Create Enum Type Specialization.vi

Filter for Enum Controls.vi

Get Measurement Configuration Controls.vi

Get Configuration MetaData.vi

Get Results MetaData.vi

Get MetaData from Configurations.vi

Get MetaData from Results.vi

Get Type Specialization.vi

Create Enum Type Specialization:

• We are always adding enum.values annotation even if a control isn't an enum. If it isn't an enum, the list of enum values is empty so having the annotation at all is confusing. I would prefer not to generate any enum annotations if the control isn't an enum.
• Actually.... it looks like you are only calling this VI after you have already filtered the list of controls to enums. In that case, you should update the control ref in/out to an enum control reference and delete all of the code that validates the control is an enum.
• Rather than require the caller to pass in the name of the parameter by extracting the text for the label of the control, just move it into the implementation of the VI.
• You shouldn't need the upcasts to generic control class. If you want to leave them to get rid of the coercion dot (type widening) due to the implicit upcast by LV for the wire tunnel, I won't complain. However, I don't generally bother with explicit upcasts just to get rid of the coercion dot. This comment likely applies throughout.
• Consider creating IsEnumControl, IsArrayControl, or IsEnumOrEnumArrayControl helper VIs for readability.
• Make this helper private like the rest of them.

Get Results Metadata:

• This is using the control for the configuration rather than the results. If we haven't already, we should test enum indicators work as expected.
• Consolidate logic between this VI and Get Configuration Metadata. Either pass in the control reference for the metadata or create a shared sub VI for the common parts.

Actually.... it looks like you are only calling this VI after you have already filtered the list of controls to enums. In that case, you should update the control ref in/out to an enum control reference and delete all of the code that validates the control is an enum.

I don't think we can use an enum control reference here as we pass in both controls references of both enum and arrays of enums as valid input.

This is using the control for the configuration rather than the results.

Are you referring to ConfigurationParameter_AnnotationsEntry.ctl being used in both Get Configuration Metadata.vi and Get Results Metadata.vi? I think this is really just a naming problem as the AnnotationsEntry gets used by both ConfigurationParameter.ctl and Output.ctl now and should be generically named. I don't believe we are incorrectly using ConfigurationParameter.ctl as that is handled at the Get MetaData from Results.vi level rather than here in the Get Results Metadata.vi which is agnostic of that type.

If we haven't already, we should test enum indicators work as expected.

I don't have a fully working example that behaviors correctly in InstrumentStudio yet since there's an issue with serialization/deserialization (which I believe needs to be resolved outside of this LabView code), but I have tested a service with a debugger to verify that an example with an enum will run through this code and produce annotations.

For all other comments, I believe I have resolved them as suggested. @jasonmreding

Screenshots below:

Create Enum Type Specialization.vi

Filter for Enum Controls.vi

Get Configuration Metadata.vi

Get Enum Type Specialization.vi

Get Measurement Configuration Controls.vi

Get MetaData from Configurations.vi

Get MetaData from Results.vi

Get Metadata.vi

Get Results Metadata.vi

IsEnumArrayControl.vi

IsEnumControl.vi

IsEnumOrEnumArrayControl.vi

I don't think we can use an enum control reference here as we pass in both controls references of both enum and arrays of enums as valid input.

The array control also has a control reference for the array element, which we get when generating the enum specialization. What I was suggesting is that the Filter for Enum Controls.vi extract this control reference for array controls (casted as enum control reference) and the original control reference for enum controls (casted as enum reference). I think if the various helpers for Is Enum, Is Enum Array, and Is Enum or Enum Array had an output for the casted reference type this would be a little easier and clean up other parts of the diagram throughout. The control ref out could be changed to the more specific type, but I think this might be more confusing than just having a second ref out for the more specific type of control reference. I still like having the Boolean output since it saves the caller from having to check/call Not A Refnum.

Are you referring to ConfigurationParameter_AnnotationsEntry.ctl being used in both Get Configuration Metadata.vi and Get Results Metadata.vi? I think this is really just a naming problem as the AnnotationsEntry gets used by both ConfigurationParameter.ctl and Output.ctl now and should be generically named. I don't believe we are incorrectly using ConfigurationParameter.ctl as that is handled at the Get MetaData from Results.vi level rather than here in the Get Results Metadata.vi which is agnostic of that type.

I'm referring to this which is called from both Get Configuration Metadata and Get Results Metadata. If I'm reading this correctly, that means we're going to generate enum annotations based on the configuration controls in both cases.

• Get MetaData from Results and Get MetaData from Configuration appear to be entirely duplicated minus the sub VI call to get the metadata that is specific to either the configuration or results. I would recommend refactoring so that either the annotations are passed into both or tunnel through the VI refnum for the appropriate ctl so the sub VI is no longer specific to configuration or results.
• IsEnumArrayControl should use IsEnumControl in its implementation.

Whoops, I accidentally re-requested you. Let me address these comments first.

Get MetaData from Results and Get MetaData from Configuration appear to be entirely duplicated minus the sub VI call to get the metadata that is specific to either the configuration or results. I would recommend refactoring so that either the annotations are passed into both or tunnel through the VI refnum for the appropriate ctl so the sub VI is no longer specific to configuration or results.

@jasonmreding I addressed this by refactoring the Get MetaData from Results.vi and Get MetaData from Configuration.vi so that we call into the non-specific Get Metadata.vi and allows us to remove both the specific Get Configuration Metadata.vi and Get Results Metadata.vi as well as creating a Get Annotations For Paramater.vi.

I addressed the other comments as suggested.

Also note the additional change to LabVIEW datatype to Measurement datatype.vi so that instead of coercing LabVIEW enums to UInts, we use the gRPC TYPE_ENUM. This was something Everett and I ran into while trying to get a fully functional example with these changes.

IsEnumArrayControl.vi

IsEnumControl.vi

Filter for Enum Controls.vi

Create Enum Type Specialization.vi

Get Enum Type Specialization.vi

Get MetaData from Configuration.vi

Get MetaData from Results.vi

Get Metadata.vi

Get Annotations For Parameter.vi

LabVIEW datatype to Measurement datatype.vi

Are there any examples that are already specifying enum type specializations that we should remove them from?

Are there any examples that are already specifying enum type specializations that we should remove them from?

No, existing examples do not use enums so there's nothing to update. I have an example locally that I've been using for testing and debugging. There was discussion about wanting to add a standalone enum example, but that might not be necessary now that we automatically create enum annotations since the user doesn't need to do anything special.

IsEnumArrayControl:

• Consider passing out the enum control reference for the array element instead of the array reference itself. This will simplify the caller a bit.

Get Enum Type Specializations:

• Minor: There are a couple of unbundle by name primitives where the Annotations property isn't wired which is kind of confusing. It would be better to just remove that element from the unbundle.

Get Annotations for Parameter:

• Notice the coercion dot on the indicator. That is because we basically have two typedefs for the same thing. One of them comes from ConfigurationParameter_AnnotationsEntry.ctl and the other comes from the grpc-labview generated typedef defined by the v2 measurement service interface. Paul tried to remove any direct dependencies to a version specific interface at this level so here's what I think we should do:
  • Rename ConfigurationParameter_AnnotationsEntry.ctl to Parameter_AnnotationsEntry.ctl.
  • Update Output.ctl to use this typedef rather than the one from the v2 measurement interface.
  • While you're at it, rename Output.ctl to OutputParameter.ctl for consistency.

IsEnumArrayControl: Consider passing out the enum control reference for the array element instead of the array reference itself. This will simplify the caller a bit.

@jasonmreding I've opted not to make any changes for this comment. It seems weird to have IsEnumControl.vi return two references to the same control and change IsEnumArrayControl.vi to return two references to two different controls. Additionally, we are cleaning up the reference to the array element in IsEnumArrayControl.vi and if instead we passed it out as an output, we would move that responsibility into the caller to have to close both the enum and the array reference. From a usage perspective, it seems more intuitive to have them behave the same.

Get Enum Type Specialization.vi

Parameter_AnnotationsEntry.ctl

OutputParameter.ctl

ConfigurationParameter.ctl

GetMetadataResponse.ctl, MeasurementSignature.ctl, Get Metadata.vi, Get MetaData from Results.vi, and Get Annotations for Parameter.vi have no visible changes, but they were just updated to reference the renamed OutputParameter.ctl and Parameter_AnnotationsEntry.ctl.